Expose mappings of requirements, provides and modules to modules #206

hikinggrass · 2024-10-06T08:08:01Z

EVerest modules and even individual interface implementations can have mappings
assigned to them. These mappings are inspired by the OCPP 3-tier model.

Modules can access the mapping information in the following ways depending on which specific information is required, all of these return an optional Mapping struct
If the mapping of a requirement is of interest it can be accessed via a get_mapping() function of a requirments:
r_name_of_the_requirement->get_mapping()

If the mapping of an interface implementation is of interest it can also be accessed via a get_mapping() function: p_name_of_an_implementation->get_mapping()

If the mapping of the current module is of interest it can be accessed via the module info:
this->info.mapping

Mapping information is also available in error reporting via "error.origin.mapping":

const auto error_handler = [this](const Everest::error::Error& error) {
    const auto evse_id = error.origin.mapping.has_value() ? error.origin.mapping.value().evse : 0;
};

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Pietfried

lgtm. Linting is currently failing

Tested with EVerest/everest-core#872

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

a-w50

I would like to request some documentation in the commit message about what has been implemented here.

a-w50 · 2024-10-15T19:29:51Z

include/utils/types.hpp

+    if (mapping.has_value()) {
+        os << mapping.value();
+    } else {
+        os << "Mapping(charging station)";


Does this default representation make any sense?

The feature (at the moment at least) is inspired by the 3-tier mappings from OCPP. So if you have no explicit mapping it's assumed that the module maps to the whole charging station, getting more specific by a mapping to an evse and/or connector

I see. Still I'm unsure, whether the string Mapping(charging station) on a text console will help users to understand what is meant.

include/utils/types.hpp

hikinggrass · 2024-10-16T15:37:34Z

I would like to request some documentation in the commit message about what has been implemented here.

Added to the PR description which will be in the squashed commit message, it's also already available in the documentation: https://everest.github.io/nightly/general/05_existing_modules.html#tier-module-mappings (the code snippets didn't render, should be fixed soon)

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

a-w50

See the comments, please address function signatures and if possible remove the mapping from the Requirement struct.

include/framework/ModuleAdapter.hpp

include/framework/everest.hpp

a-w50 · 2024-10-17T10:46:31Z

include/utils/config.hpp

@@ -147,9 +147,9 @@ class Config {
    json resolve_requirement(const std::string& module_id, const std::string& requirement_id) const;

    ///
-    /// \returns a list of Requirements for \p module_id
+    /// \returns a map of Fulfillments for \p module_id


I really doubt the value of these comments, you get exactly the same information if you look at the function signature.

include/utils/config.hpp

include/utils/types.hpp

lib/config.cpp

a-w50 · 2024-10-17T11:10:45Z

lib/config.cpp

    if (mapping.implementations.find(impl_id) == mapping.implementations.end()) {
-        return std::nullopt;
+        // if no specific implementation mapping is given, use the module mapping
+        return mapping.module;
    }
    return mapping.implementations.at(impl_id);


This pattern happens quite often. It is usually better to use the iterator instead of doing the lookup twice:

const auto mapping_it = ...find(impl_id); if (mapping_id == std::end(...)) { return default; } return *mapping_it;

I guess something like this would be quite readable:

const auto& mappings = module_tier_mappings.value(); const auto mapping_it = mappings.implementations.find(impl_id); if (mapping_it == mappings.implementations.end()) { return mappings.module; } const auto& [key, mapping] = (*mapping_it); return mapping;

The alternative doesn't quite feel as readable:

... return (*mapping_it).second;

Or what do you think?

lib/everest.cpp

Functionality now provided via a free helper function with the same name Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

re-introduce get_requirements function Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

… a different type Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

lib/config.cpp

include/utils/types.hpp

lib/runtime.cpp

…equirements and get_fulfillments Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

These can be used in (generated) user code to initialize requirements with their fulfillments and optional mappings Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

include/utils/types.hpp

a-w50 · 2024-10-21T13:51:29Z

include/utils/types.hpp

    std::optional<Mapping> mapping;
 };

+using RequirementInitialization = std::map<std::string, std::vector<RequirementInitializer>>;


I'm uncertain with that name: RequirementInitialization sounds more like the result of the initialization of a requirement than a mapping from each requirement name to its initializer.

I'm open for suggestions 😄 that was the best I could come up with at the time 😅

lib/config.cpp

a-w50 · 2024-10-21T13:58:50Z

lib/config.cpp

+
+    for (const auto& [requirement, fulfillment] : this->resolve_requirements(module_id)) {
+        res.push_back(requirement);
+        (void)fulfillment; // fulfillment is not used here


This void is a not that beautiful. [[maybe_unused]] could be used? Or just normal it->first?

a-w50 · 2024-10-21T13:59:52Z

lib/config.cpp

+    return res;
+}
+
+RequirementInitialization Config::get_requirement_initialization(const std::string& module_id) const {


This function is now very readable 👍

a-w50 · 2024-10-21T14:02:50Z

lib/config.cpp

+
+    for (const auto& [requirement, fulfillment] : this->resolve_requirements(module_id)) {
+        const auto& mapping = this->get_3_tier_model_mapping(fulfillment.module_id, fulfillment.implementation_id);
+        res[requirement.id].push_back({requirement, fulfillment, mapping});


This is a bit tricky. We implicitly rely on the fact, that requirements are sorted within the map according to their index. This should be guaranteed, but still is not obvious to the reader. Having the requirement without it's index, but instead pointing at a vector of Fulfillments would probably make the code even simpler.

include/utils/types.hpp

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

hikinggrass added 4 commits October 4, 2024 17:52

WIP: make mappings available outside of errors

630c48c

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Add stream operators for (optional) Mapping for easier debugging

0105d03

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Add module mapping to module info

b67b7b1

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Bump version to 0.18

9dd6f7f

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

This was referenced Oct 6, 2024

Expose mappings of requirements, provides and modules to modules EVerest/everest-utils#154

Draft

Add documentation about the Mapping feature EVerest/EVerest#198

Merged

Pietfried mentioned this pull request Oct 10, 2024

Refactor Energy Management integration of OCPP and API modules EVerest/everest-core#872

Open

3 tasks

Pietfried approved these changes Oct 14, 2024

View reviewed changes

hikinggrass force-pushed the feature/expose-mappings branch from 131e44c to 9a4ffc1 Compare October 15, 2024 15:15

Merge remote-tracking branch 'origin/main' into feature/expose-mappings

ad32fd1

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

hikinggrass force-pushed the feature/expose-mappings branch from 9a4ffc1 to ad32fd1 Compare October 15, 2024 15:19

clang-format

557932c

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

a-w50 requested changes Oct 15, 2024

View reviewed changes

Pietfried added the include-in-release label Oct 16, 2024

Rename RequirementConnection to Fulfillment

9edf536

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

hikinggrass requested a review from a-w50 October 16, 2024 16:30

clang-format

e90e60f

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

a-w50 requested changes Oct 17, 2024

View reviewed changes

hikinggrass added 6 commits October 17, 2024 17:59

Remove get_imple_mapping from ModuleAdapter

7d0b0e4

Functionality now provided via a free helper function with the same name Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Move mapping member from Requirement to Fulfilment

acac5bd

re-introduce get_requirements function Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Rename function so it doesn't share a name with another one returning…

9877ce6

… a different type Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Make more mapping related variables&functions const

b130124

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Add reworded comment

44baf44

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

clang-format

501f01f

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

hikinggrass requested a review from a-w50 October 17, 2024 17:26

a-w50 requested changes Oct 18, 2024

View reviewed changes

lib/config.cpp Outdated Show resolved Hide resolved

lib/config.cpp Outdated Show resolved Hide resolved

include/utils/types.hpp Outdated Show resolved Hide resolved

lib/runtime.cpp Outdated Show resolved Hide resolved

hikinggrass added 3 commits October 18, 2024 12:36

Add resolve_requirements that is used as a basis for refactored get_r…

8c3feca

…equirements and get_fulfillments Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Remove code that isn't used

61b9281

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Introduce RequirementInitializer and RequirementInitialization

5d542d3

These can be used in (generated) user code to initialize requirements with their fulfillments and optional mappings Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

hikinggrass added 2 commits October 21, 2024 11:21

Remove redundant mapping from Fulfillment

26473f7

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Turn operator< of Requirement into free function

000efd5

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

a-w50 reviewed Oct 21, 2024

View reviewed changes

hikinggrass added 4 commits October 21, 2024 17:24

Remove constructors of Requirement

cd20aab

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Remove ugly void cast

6084d34

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

More const auto(&) usage

59644d3

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

Fix everestrs build after removal of Requirement constructor

2f45d45

Signed-off-by: Kai-Uwe Hermann <kai-uwe.hermann@pionix.de>

hikinggrass removed the include-in-release label Oct 23, 2024

a-w50 self-requested a review October 23, 2024 14:52

a-w50 approved these changes Oct 23, 2024

View reviewed changes

Merge branch 'main' into feature/expose-mappings

b0fc647

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose mappings of requirements, provides and modules to modules #206

Expose mappings of requirements, provides and modules to modules #206

hikinggrass commented Oct 6, 2024 •

edited

Loading

Pietfried left a comment •

edited

Loading

a-w50 left a comment •

edited

Loading

a-w50 Oct 15, 2024

hikinggrass Oct 16, 2024

a-w50 Oct 17, 2024

hikinggrass commented Oct 16, 2024

a-w50 left a comment

a-w50 Oct 17, 2024

a-w50 Oct 17, 2024

hikinggrass Oct 17, 2024

a-w50 Oct 21, 2024

hikinggrass Oct 21, 2024

a-w50 Oct 21, 2024

a-w50 Oct 21, 2024

a-w50 Oct 21, 2024

Expose mappings of requirements, provides and modules to modules #206

Are you sure you want to change the base?

Expose mappings of requirements, provides and modules to modules #206

Conversation

hikinggrass commented Oct 6, 2024 • edited Loading

Pietfried left a comment • edited Loading

Choose a reason for hiding this comment

a-w50 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hikinggrass commented Oct 16, 2024

a-w50 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hikinggrass commented Oct 6, 2024 •

edited

Loading

Pietfried left a comment •

edited

Loading

a-w50 left a comment •

edited

Loading